Skip to content

Agentic Review: Baseline comparison#34926

Merged
ghengeveld merged 17 commits into
yann/agentic-review-mcp-integrationfrom
review-baseline-comparison
Jun 4, 2026
Merged

Agentic Review: Baseline comparison#34926
ghengeveld merged 17 commits into
yann/agentic-review-mcp-integrationfrom
review-baseline-comparison

Conversation

@ghengeveld

@ghengeveld ghengeveld commented May 27, 2026

Copy link
Copy Markdown
Member

What I did

Adds a baseline-vs-latest comparison experience to the Agentic Review Details screen:

  • Dual preview: renders the baseline and latest story side-by-side. Toggle between 1up (single, overlaid) and 2up (side-by-side) layouts; in single mode a swap control flips which side is shown.
  • Readiness gating: latest stays visible while the baseline preview loads, so there's no flash of empty/broken state. The bottom toolbar only appears once the baseline is ready.
  • Synced iframes: baseline and latest iframes are kept aligned by syncing URL and scroll position.
  • Baseline proxy: baseline previews are proxied through a local /__review-baseline dev-server route (experimental_devServer + http-proxy-middleware) to avoid cross-origin issues.
  • Renames the GuidePage header from "Guide" to "Onboarding guide".
  • Updates DetailsScreen stories/play functions to target the Guide Page story.

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

  1. Run the internal Storybook UI (cd code && yarn storybook:ui) with the review addon enabled.
  2. Open the Agentic Review Details screen for a changed story.
  3. Confirm the baseline preview loads via the /__review-baseline proxy and the latest preview stays visible while it loads.
  4. Toggle between 1up and 2up layouts; in 1up use the swap control to flip between baseline and latest.
  5. Scroll one preview and confirm the other stays aligned (URL + scroll position sync).

Documentation

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli-storybook/src/sandbox-templates.ts

  • Make sure this PR contains one of the labels below:

    Available labels
    • bug: Internal changes that fixes incorrect behavior.
    • maintenance: User-facing maintenance tasks.
    • dependencies: Upgrading (sometimes downgrading) dependencies.
    • build: Internal-facing build tooling & test updates. Will not show up in release changelog.
    • cleanup: Minor cleanup style change. Will not show up in release changelog.
    • documentation: Documentation only changes. Will not show up in release changelog.
    • feature request: Introducing a new feature.
    • BREAKING CHANGE: Changes that break compatibility in some way with current major version.
    • other: Changes that don't fit in the above categories.

🦋 Canary release

This pull request has been released as version 0.0.0-pr-34926-sha-a0498164. Try it out in a new sandbox by running npx storybook@0.0.0-pr-34926-sha-a0498164 sandbox or in an existing project with npx storybook@0.0.0-pr-34926-sha-a0498164 upgrade.

More information
Published version 0.0.0-pr-34926-sha-a0498164
Triggered by @yannbf
Repository storybookjs/storybook
Branch review-baseline-comparison
Commit a0498164
Datetime Thu May 28 12:17:46 UTC 2026 (1779970666)
Workflow run 26574136441

To request a new release of this pull request, mention the @storybookjs/core team.

core team members can create a new canary release here or locally with gh workflow run --repo storybookjs/storybook publish.yml --field pr=34926

Summary by CodeRabbit

  • New Features

    • Baseline vs Latest preview with synchronized scrolling (side-by-side and single-view modes)
    • Toggle between single and dual preview layouts; swap visible preview in single mode
    • Experimental local proxy to reliably load baseline previews during review
  • Chores

    • Onboarding header updated to “Onboarding guide”
    • Review details updated to target the Guide Page story and align story behaviors

@ghengeveld ghengeveld requested a review from yannbf May 27, 2026 09:01
@ghengeveld ghengeveld self-assigned this May 27, 2026
@ghengeveld ghengeveld added feature request ci:normal Run our default set of CI jobs (choose this for most PRs). labels May 27, 2026
@ghengeveld ghengeveld force-pushed the review-baseline-comparison branch from 4be9f90 to ad35795 Compare May 27, 2026 09:02
@coderabbitai

coderabbitai Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR adds baseline-vs-latest preview comparison to the Review addon: an experimental devServer preset that proxies baseline requests via /__review-baseline, a refactored DetailsScreen with dual iframes, synchronized scrolling and 1-up/2-up preview modes, and a GuidePage title update.

Changes

Baseline-vs-Latest Review Comparison

Layer / File(s) Summary
Baseline Proxy Middleware Setup
code/addons/review/src/preset.ts, code/addons/review/package.json
New experimental_devServer preset registers an HTTP proxy at /__review-baseline that forwards requests to the Chromatic origin with path-prefix stripping. Adds http-proxy-middleware devDependency.
DetailsScreen Baseline-vs-Latest Viewer
code/addons/review/src/screens/DetailsScreen.tsx
Converted to hooks-based component rendering baseline and latest iframes, adds PreviewMode (1-up/2-up) and VisibleSide, styled layout primitives, bidirectional scroll sync, baseline URL generation via proxy, STORY_RENDERED listener hookup, and dynamic toolbar controls for mode/swap actions.
GuidePage Text Update
code/core/src/manager/settings/GuidePage.tsx
GuidePage <h1> text changed from "Guide" to "Onboarding guide".

Sequence Diagram(s)

sequenceDiagram
  participant DevServerApp
  participant review_preset as review/src/preset.ts
  participant ChromaticOrigin
  DevServerApp->>review_preset: mount experimental_devServer(app)
  review_preset->>DevServerApp: register proxy at /__review-baseline
  DevServerApp->>ChromaticOrigin: proxied request (path rewritten, changeOrigin)
Loading
sequenceDiagram
  participant DetailsScreen
  participant LatestIframe as Latest Preview Iframe
  participant BaselineIframe as Baseline Preview Iframe
  participant Addons as Storybook Addons Channel
  LatestIframe->>DetailsScreen: load/navigation event
  DetailsScreen->>DetailsScreen: compute baseline proxy URL (toBaselinePreviewUrl)
  DetailsScreen->>BaselineIframe: set src to proxy URL
  BaselineIframe->>Addons: STORY_RENDERED
  Addons->>DetailsScreen: deliver STORY_RENDERED
  DetailsScreen->>DetailsScreen: setup bidirectional scroll sync
  DetailsScreen->>LatestIframe: apply scroll position
  DetailsScreen->>BaselineIframe: apply scroll position
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • storybookjs/storybook#32862: Both PRs implement the experimental_devServer preset mechanism; core PR exposes/apply the new preset while this addon mounts the /__review-baseline proxy.
  • storybookjs/storybook#32816: The addon’s experimental_devServer(app) preset depends on core infrastructure introduced in this PR to expose the dev server to presets/addons.

Comment @coderabbitai help to get the list of available commands and usage tips.

@storybook-app-bot

storybook-app-bot Bot commented May 27, 2026

Copy link
Copy Markdown

Package Benchmarks

Commit: 52e8d0a, ran on 3 June 2026 at 14:04:42 UTC

The following packages have significant changes to their size or dependencies:

@storybook/addon-review

Before After Difference
Dependency count 0 0 0
Self size 53 KB 206 KB 🚨 +153 KB 🚨
Dependency size 654 B 654 B 0 B
Bundle Size Analyzer Link Link

Base automatically changed from agentic-review-design-iterations to yann/agentic-review-mcp-integration May 28, 2026 08:04
@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Actionable comments posted: 0

1 similar comment
@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Actionable comments posted: 0

…screen with 1up/2up modes, view toggles, and readiness gating so latest stays visible while baseline loads. Also proxy baseline previews through a local __review-baseline dev-server route and keep the two iframes aligned by syncing URL and scroll position.
@ghengeveld ghengeveld force-pushed the review-baseline-comparison branch from 8f1fff5 to b677c0a Compare May 29, 2026 10:03

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@code/addons/review/src/preset.ts`:
- Around line 79-87: The proxy created in createProxyMiddleware (assigned to
proxyRequest) lacks timeout and deterministic error handling; update the options
passed to createProxyMiddleware (for the instance referenced as proxyRequest and
mounted via app.use(BASELINE_PROXY_PATH)) to include timeout and proxyTimeout
values (e.g., sensible millisecond limits), and add an on.error handler that
sends a deterministic HTTP failure (e.g., 502/504 with a short JSON/plain text
body) and ends the response to avoid hanging the request when
BASELINE_TARGET_ORIGIN is slow/unreachable; keep existing changeOrigin and
pathRewrite behavior and ensure the handler checks res.headersSent before
writing a response.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2c695d1e-0cc1-4f59-a8c6-bba28ad492a6

📥 Commits

Reviewing files that changed from the base of the PR and between 8f1fff5 and b677c0a.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (5)
  • code/addons/review/package.json
  • code/addons/review/src/preset.ts
  • code/addons/review/src/screens/DetailsScreen.stories.tsx
  • code/addons/review/src/screens/DetailsScreen.tsx
  • code/core/src/manager/settings/GuidePage.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
  • code/core/src/manager/settings/GuidePage.tsx
  • code/addons/review/src/screens/DetailsScreen.stories.tsx
  • code/addons/review/src/screens/DetailsScreen.tsx

Comment thread code/addons/review/src/preset.ts
@ghengeveld ghengeveld added the qa:needed Pull Requests that will need manual QA prior to release. label Jun 2, 2026
yannbf and others added 3 commits June 3, 2026 00:07
Guard documentElement before setting overscroll styles in DetailsScreen,
since the capture-loopback baseline iframe can expose a document with a
null documentElement, throwing "Cannot read properties of null (reading
'style')" during the Details story.

Update the SummaryScreen Minimal play function to match the current
generic headline ("Showing N agent-curated stories...") instead of the
removed branch-specific text.

Co-authored-by: Cursor <cursoragent@cursor.com>
The baseline preview proxies to a remote Chromatic origin that can be
slow or unreachable. Bound the wait with timeout/proxyTimeout and return
a deterministic 502 via on.error so a dead connection fails fast instead
of hanging the review UI's baseline iframe.

Co-authored-by: Cursor <cursoragent@cursor.com>
@ghengeveld ghengeveld force-pushed the review-baseline-comparison branch from 6332063 to 42403d1 Compare June 2, 2026 22:08
ghengeveld added 11 commits June 3, 2026 09:18
… prop

The Details play function asserted on "Latest on <branch>" text, a branch
footer that was removed when branch-specific copy was dropped from the
review UI. Assert on the always-rendered latest preview iframe instead,
matching DetailsScreen.stories, and remove the now-unused branchName prop
that was threaded into DetailsScreen but never rendered.
Fetch the baseline Storybook's index.json (via the existing review proxy,
keyed on the review's createdAt) and flag stories that don't exist in the
baseline with a positive "New" badge in the DetailsScreen toolbar. No badge
shows when the baseline is unavailable or the story already exists.
- Hide the baseline preview, side-by-side toggle and bottom comparison
  bar for newly added stories (no baseline to compare against).
- Render the baseline pane and comparison bar immediately since baseline
  existence is known up front, instead of waiting for the iframe onload.
- Default to the 2-up side-by-side layout and remember the user's
  1up/2up choice across detail/summary navigation via sessionStorage.
Centralize all sessionStorage usage in a single session-store utility that
guards every access (privacy modes, disabled storage, sandboxed iframes can
throw), so persistence degrades gracefully and in-memory state still drives
the current session.
The detail screen now treats a story as new when the change-detection
mechanism flags it (status-value:new), in addition to the existing
baseline-absence check. This surfaces newly added stories even when a
baseline exists or is unavailable.
- Remove unsupported branchName prop passed to DetailsScreen in the
  ReviewFlowDemo prototype story (DetailsScreen never consumed it).
- Anchor ReviewPage story createdAt to Date.now() so the "Created … ago"
  label renders as a small, stable value instead of computing minutes
  against a fixed past timestamp (caused a huge number in Chromatic).
Revert the summary header to a non-semantic clickable row (a pointer
affordance only) and move the accessible toggle back to the chevron
<Button>, which carries the aria-label and now aria-expanded state for
assistive tech. This also restores the chevron's column alignment under
the toolbar's expand/collapse-all button without the manual margin hack.
When opening the detail screen, focus stayed on the now-hidden summary
because React 18 drops a boolean `inert` prop. Set the DOM property
imperatively so the mounted summary leaves the tab order and a11y tree,
and focus the back control when the detail screen mounts.
Focusing the heading orients screen-reader users by what they opened
rather than landing on an action; the heading is a programmatic focus
target (tabindex=-1) with no focus ring.
The "Find stories" input had no visible focus indicator. Mirror the
sidebar search: the wrapper owns the focus ring while the input stays
outline-less.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:normal Run our default set of CI jobs (choose this for most PRs). feature request qa:needed Pull Requests that will need manual QA prior to release.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants